oran-o2ims: add tls scanner - #82154
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe ORAN O2IMS CI configuration adds a TLS scanner image and optional validation job. The scanner now accepts an existing namespace through ChangesORAN O2IMS TLS validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant CIJob
participant OperatorBundle
participant OpenShiftAPI
CIJob->>OperatorBundle: Install operator-bundle in oran-o2ims
CIJob->>OpenShiftAPI: Wait for Inventory and discover annotated services
OpenShiftAPI-->>CIJob: Return pod selectors and TLS secret names
CIJob->>OpenShiftAPI: Wait for selected pods to become Ready
CIJob->>OpenShiftAPI: Verify referenced TLS secrets
CIJob->>CIJob: Run tls-13 and tls-scanner-run
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @rauhersu. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@donpenney , may you run an ok-to-test ? |
|
/ok-to-test |
|
/retest |
1 similar comment
|
/retest |
|
/test ci-operator-config |
|
/test generated-config |
|
/pj-rehearse auto-ack |
|
@rauhersu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
The new TLS scanner - Example for this execution: After this PR merges, then on any PR to openshift-kni/oran-o2ims, someone can trigger it with @Missxiaoguo @donpenney , I would need a lgtm+approve if you agree with these changes. |
|
Since one of my PRs is referenced here - if you are looking for TLS Adherence test this setup won't do. If you're looking for plain compliance check it's fine. I later found out that I missed it in the AWS tls scan job referenced here and had to amend it to get the scanner check TLS adherence which is what I was looking for. In other words |
|
Need to assess the new @RomanBednar comment. Thanks, Roman ! /hold |
a6c6c37 to
3ce5266
Compare
|
/pj-rehearse auto-ack |
|
@rauhersu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
3ce5266 to
0d5f3d0
Compare
|
/retest |
|
@rauhersu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse auto-ack |
2 similar comments
|
/pj-rehearse auto-ack |
|
/pj-rehearse auto-ack |
|
@rauhersu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
1 similar comment
|
@rauhersu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
e42884d to
9f666e5
Compare
|
/pj-rehearse auto-ack |
|
@rauhersu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Signed-off-by: Raul Hernandez <rauherna@redhat.com>
- Add a wait-for-server-pods inline step to the install-bundle-tls-scan test that dynamically discovers TLS-serving services via the service.beta.openshift.io/serving-cert-secret-name annotation, waits for their backing pods to become Ready, and verifies that service-ca TLS secrets exist before the tls-scanner runs. This avoids hardcoding server names. - Move operator.substitutions under the bundle item where it belongs, making the pullspec override explicitly scoped to the operator-bundle.
…space The scanner always runs in a dedicated tls-scanner namespace. When scanning pods with NetworkPolicies restricting ingress to same-namespace traffic (e.g. oran-o2ims servers), the TLS handshake probes are blocked, producing false NO_TLS results. Add a SCANNER_NAMESPACE env var that deploys the scanner pod into an existing namespace instead, reusing the OWNS_NAMESPACE=false pattern already used by the HyperShift management-cluster path. Set SCANNER_NAMESPACE=oran-o2ims in the install-bundle-tls-scan test so the scanner satisfies the oran-o2ims NetworkPolicy rules.
When the scanner pod fails (e.g. TLS compliance violations), the script exits before reaching the trap unregistration at the end of run_tls_scan. The EXIT trap fires the cleanup function, but bash unwinds the function call stack before running the trap handler. Since NAMESPACE and OWNS_NAMESPACE were declared local to run_tls_scan, they no longer exist in the trap handler's scope, causing a fatal "unbound variable" error under set -o nounset. Fix by removing the local qualifier from both variables. They are safe as globals: the non-HyperShift path calls the function once, and the HyperShift path runs each invocation in a subshell that isolates variable state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Raúl Hernández <rauherna@redhat.com>
Upstream PR openshift#82522 fixed the cleanup trap crash by snapshotting locals into _CLEANUP_* globals. Restore NAMESPACE and OWNS_NAMESPACE to local (matching upstream) while keeping the SCANNER_NAMESPACE logic from our earlier commits. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Raúl Hernández <rauherna@redhat.com>
Add the install-bundle-tls-scan test with TLS Adherence (StrictAllComponents) and tls-scanner to the release-4.22 config, matching the main branch configuration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Raúl Hernández <rauherna@redhat.com>
Add install-bundle-tls-scan-periodic to both main (Fridays 03:00 UTC) and release-4.22 (Mondays 03:00 UTC) configurations. The periodic jobs are identical to the on-demand variants but run on a cron schedule. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Raúl Hernández <rauherna@redhat.com>
94b327b to
e2190d8
Compare
|
/pj-rehearse auto-ack |
|
@rauhersu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-kni-oran-o2ims-release-4.22-install-bundle-tls-scan-periodic |
|
@rauhersu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-cert-manager-operator-master-tls-scanner |
|
@rauhersu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@rauhersu: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Set PQC_CHECK=true across all TLS scan pipelines (on-demand and periodic) for main and release-4.22. This enables post-quantum cryptography readiness checks (TLS 1.3 + mlkem/mlkem25519 support). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Raúl Hernández <rauherna@redhat.com>
|
/pj-rehearse auto-ack |
|
@rauhersu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
A total of 135 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
Summary
Adds TLS scanner CI integration for
openshift-kni/oran-o2ims.The new
install-bundle-tls-scanjob:ipi-awsworkflow.operator-sdk run bundlevia theoptional-operators-operator-sdkstep-registry ref.InventoryCR on startup, which triggers the reconciler to deploy all server pods.tls-scanner-runstep-registry ref to scan all TLS endpoints in theoran-o2imsnamespace.The job is configured as
always_run: falseandoptional: true, so it does not run automatically on every PR and does not block merges. It can be triggered on demand with/test install-bundle-tls-scan.Changes
ci-operator/config/openshift-kni/oran-o2ims/openshift-kni-oran-o2ims-main.yaml: Addedtls-scanner-toolbase image and theinstall-bundle-tls-scantest definition.ci-operator/jobs/openshift-kni/oran-o2ims/openshift-kni-oran-o2ims-main-presubmits.yaml: Auto-generated bymake jobs.References
Summary by CodeRabbit
Adds optional TLS scanner CI integration for the
openshift-kni/oran-o2imscomponent in the OpenShift CI infrastructure configuration (ci-operator/config/openshift-kni/oran-o2ims/openshift-kni-oran-o2ims-main.yaml).tls-scanner-toolbase image entry used by the TLS scanning workflow.install-bundle-tls-scan(always_run: false,optional: true, triggerable via/test install-bundle-tls-scan) that:oran-o2imsnamespace (viaoperator-sdk run bundle);inventory/defaultCR;service.beta.openshift.io/serving-cert-secret-name, derives pod label selectors from each Service’sspec.selector, and waits up to 5 minutes for matching pods to becomeReady(logging pod status/events on timeout);oran-o2imsnamespace;tls-13workflow step before executingtls-scanner-run.tls-scanner-run-ref.yamladdsSCANNER_NAMESPACE(empty by default); when set, the scanner uses the provided namespace rather than a dedicatedtls-scannernamespace.tls-scanner-run-commands.shderives the scannerNAMESPACEfromSCANNER_NAMESPACEand disables “owns namespace” behavior wheneverSCANNER_NAMESPACEis explicitly set.